From 45ce6547e6bde8cdf8f3dcaba833a3bda8f3c889 Mon Sep 17 00:00:00 2001 From: "kfraser@localhost.localdomain" Date: Mon, 29 Jan 2007 10:52:17 +0000 Subject: [PATCH] Replace sprintf with snprintf and strncpy with strlcpy. There are various cases where no NULL-terminated strings are guaranteed and eventual possible overflows. This patch fixes them. From: Christoph Egger Signed-off-by: Keir Fraser --- xen/arch/x86/cpu/centaur.c | 3 +- xen/arch/x86/cpu/common.c | 4 +-- xen/arch/x86/domain_build.c | 4 +-- xen/arch/x86/hvm/intercept.c | 2 +- xen/arch/x86/oprofile/nmi_int.c | 35 ++++++++++------------ xen/arch/x86/setup.c | 47 +++++++++++++++++++++--------- xen/arch/x86/time.c | 2 +- xen/common/gdbstub.c | 2 +- xen/common/kernel.c | 3 +- xen/common/keyhandler.c | 5 ++-- xen/common/libelf/libelf-dominfo.c | 16 +++++----- xen/common/perfc.c | 3 +- xen/common/rangeset.c | 5 ++-- xen/common/symbols.c | 10 ++++--- 14 files changed, 79 insertions(+), 62 deletions(-) diff --git a/xen/arch/x86/cpu/centaur.c b/xen/arch/x86/cpu/centaur.c index 09e5498c2d..7744b5b5e5 100644 --- a/xen/arch/x86/cpu/centaur.c +++ b/xen/arch/x86/cpu/centaur.c @@ -437,7 +437,8 @@ static void __init init_centaur(struct cpuinfo_x86 *c) /* Add L1 data and code cache sizes. */ c->x86_cache_size = (cc>>24)+(dd>>24); } - sprintf( c->x86_model_id, "WinChip %s", name ); + snprintf( c->x86_model_id, sizeof(c->x86_model_id), + "WinChip %s", name ); break; case 6: diff --git a/xen/arch/x86/cpu/common.c b/xen/arch/x86/cpu/common.c index 63237c82f4..ae13c7341b 100644 --- a/xen/arch/x86/cpu/common.c +++ b/xen/arch/x86/cpu/common.c @@ -386,8 +386,8 @@ void __devinit identify_cpu(struct cpuinfo_x86 *c) strcpy(c->x86_model_id, p); else /* Last resort... */ - sprintf(c->x86_model_id, "%02x/%02x", - c->x86_vendor, c->x86_model); + snprintf(c->x86_model_id, sizeof(c->x86_model_id), + "%02x/%02x", c->x86_vendor, c->x86_model); } /* Now the feature flags better reflect actual CPU features! */ diff --git a/xen/arch/x86/domain_build.c b/xen/arch/x86/domain_build.c index 5e505bf4af..b32f57861c 100644 --- a/xen/arch/x86/domain_build.c +++ b/xen/arch/x86/domain_build.c @@ -821,7 +821,7 @@ int construct_dom0(struct domain *d, si->pt_base = vpt_start + 2 * PAGE_SIZE * !!IS_COMPAT(d); si->nr_pt_frames = nr_pt_pages; si->mfn_list = vphysmap_start; - sprintf(si->magic, "xen-%i.%i-x86_%d%s", + snprintf(si->magic, sizeof(si->magic), "xen-%i.%i-x86_%d%s", xen_major_version(), xen_minor_version(), elf_64bit(&elf) ? 64 : 32, parms.pae ? "p" : ""); @@ -871,7 +871,7 @@ int construct_dom0(struct domain *d, memset(si->cmd_line, 0, sizeof(si->cmd_line)); if ( cmdline != NULL ) - strncpy((char *)si->cmd_line, cmdline, sizeof(si->cmd_line)-1); + strlcpy((char *)si->cmd_line, cmdline, sizeof(si->cmd_line)); if ( fill_console_start_info((void *)(si + 1)) ) { diff --git a/xen/arch/x86/hvm/intercept.c b/xen/arch/x86/hvm/intercept.c index 9a454c6290..e5c1728b7d 100644 --- a/xen/arch/x86/hvm/intercept.c +++ b/xen/arch/x86/hvm/intercept.c @@ -173,7 +173,7 @@ int hvm_register_savevm(struct domain *d, return -1; } - strncpy(se->idstr, idstr, HVM_SE_IDSTR_LEN); + strlcpy(se->idstr, idstr, HVM_SE_IDSTR_LEN); se->instance_id = instance_id; se->version_id = version_id; diff --git a/xen/arch/x86/oprofile/nmi_int.c b/xen/arch/x86/oprofile/nmi_int.c index ffefe3bf2d..518691337c 100644 --- a/xen/arch/x86/oprofile/nmi_int.c +++ b/xen/arch/x86/oprofile/nmi_int.c @@ -22,6 +22,7 @@ #include #include #include +#include #include "op_counter.h" #include "op_x86_model.h" @@ -39,7 +40,6 @@ extern int is_active(struct domain *d); extern int active_id(struct domain *d); extern int is_profiled(struct domain *d); -extern size_t strlcpy(char *dest, const char *src, size_t size); static int nmi_callback(struct cpu_user_regs *regs, int cpu) @@ -276,20 +276,20 @@ static int __init p4_init(char * cpu_type) } #ifndef CONFIG_SMP - strncpy (cpu_type, "i386/p4", XENOPROF_CPU_TYPE_SIZE - 1); + strlcpy (cpu_type, "i386/p4", XENOPROF_CPU_TYPE_SIZE); model = &op_p4_spec; return 1; #else switch (smp_num_siblings) { case 1: - strncpy (cpu_type, "i386/p4", - XENOPROF_CPU_TYPE_SIZE - 1); + strlcpy (cpu_type, "i386/p4", + XENOPROF_CPU_TYPE_SIZE); model = &op_p4_spec; return 1; case 2: - strncpy (cpu_type, "i386/p4-ht", - XENOPROF_CPU_TYPE_SIZE - 1); + strlcpy (cpu_type, "i386/p4-ht", + XENOPROF_CPU_TYPE_SIZE); model = &op_p4_ht2_spec; return 1; } @@ -311,17 +311,17 @@ static int __init ppro_init(char *cpu_type) return 0; } else if (cpu_model == 15) - strncpy (cpu_type, "i386/core_2", XENOPROF_CPU_TYPE_SIZE - 1); + strlcpy (cpu_type, "i386/core_2", XENOPROF_CPU_TYPE_SIZE); else if (cpu_model == 14) - strncpy (cpu_type, "i386/core", XENOPROF_CPU_TYPE_SIZE - 1); + strlcpy (cpu_type, "i386/core", XENOPROF_CPU_TYPE_SIZE); else if (cpu_model == 9) - strncpy (cpu_type, "i386/p6_mobile", XENOPROF_CPU_TYPE_SIZE - 1); + strlcpy (cpu_type, "i386/p6_mobile", XENOPROF_CPU_TYPE_SIZE); else if (cpu_model > 5) - strncpy (cpu_type, "i386/piii", XENOPROF_CPU_TYPE_SIZE - 1); + strlcpy (cpu_type, "i386/piii", XENOPROF_CPU_TYPE_SIZE); else if (cpu_model > 2) - strncpy (cpu_type, "i386/pii", XENOPROF_CPU_TYPE_SIZE - 1); + strlcpy (cpu_type, "i386/pii", XENOPROF_CPU_TYPE_SIZE); else - strncpy (cpu_type, "i386/ppro", XENOPROF_CPU_TYPE_SIZE - 1); + strlcpy (cpu_type, "i386/ppro", XENOPROF_CPU_TYPE_SIZE); model = &op_ppro_spec; return 1; @@ -346,9 +346,6 @@ int nmi_init(int *num_events, int *is_primary, char *cpu_type) } } - /* Make sure string is NULL terminated */ - cpu_type[XENOPROF_CPU_TYPE_SIZE - 1] = 0; - switch (vendor) { case X86_VENDOR_AMD: /* Needs to be at least an Athlon (or hammer in 32bit mode) */ @@ -361,15 +358,15 @@ int nmi_init(int *num_events, int *is_primary, char *cpu_type) return -ENODEV; case 6: model = &op_athlon_spec; - strncpy (cpu_type, "i386/athlon", - XENOPROF_CPU_TYPE_SIZE - 1); + strlcpy (cpu_type, "i386/athlon", + XENOPROF_CPU_TYPE_SIZE); break; case 0xf: model = &op_athlon_spec; /* Actually it could be i386/hammer too, but give user space an consistent name. */ - strncpy (cpu_type, "x86-64/hammer", - XENOPROF_CPU_TYPE_SIZE - 1); + strlcpy (cpu_type, "x86-64/hammer", + XENOPROF_CPU_TYPE_SIZE); break; } break; diff --git a/xen/arch/x86/setup.c b/xen/arch/x86/setup.c index 96d49203ca..c8daf1c28e 100644 --- a/xen/arch/x86/setup.c +++ b/xen/arch/x86/setup.c @@ -111,8 +111,7 @@ char acpi_param[10] = ""; static void parse_acpi_param(char *s) { /* Save the parameter so it can be propagated to domain0. */ - strncpy(acpi_param, s, sizeof(acpi_param)); - acpi_param[sizeof(acpi_param)-1] = '\0'; + strlcpy(acpi_param, s, sizeof(acpi_param)); /* Interpret the parameter for use within Xen. */ if ( !strcmp(s, "off") ) @@ -804,35 +803,57 @@ void __init __start_xen(multiboot_info_t *mbi) void arch_get_xen_caps(xen_capabilities_info_t info) { char *p = info; + int i = 0; int major = xen_major_version(); int minor = xen_minor_version(); #if defined(CONFIG_X86_32) && !defined(CONFIG_X86_PAE) - p += sprintf(p, "xen-%d.%d-x86_32 ", major, minor); - if ( hvm_enabled ) - p += sprintf(p, "hvm-%d.%d-x86_32 ", major, minor); + i = snprintf(p, sizeof(xen_capabilities_info_t), + "xen-%d.%d-x86_32 ", major, minor); + p += i; + if ( hvm_enabled ) { + i = snprintf(p, sizeof(xen_capabilities_info_t) - i, + "hvm-%d.%d-x86_32 ", major, minor); + p += i; + } #elif defined(CONFIG_X86_32) && defined(CONFIG_X86_PAE) - p += sprintf(p, "xen-%d.%d-x86_32p ", major, minor); + i = snprintf(p, sizeof(xen_capabilities_info_t), + "xen-%d.%d-x86_32p ", major, minor); + p += i; if ( hvm_enabled ) { - p += sprintf(p, "hvm-%d.%d-x86_32 ", major, minor); - p += sprintf(p, "hvm-%d.%d-x86_32p ", major, minor); + i = snprintf(p, sizeof(xen_capabilities_info_t), + "hvm-%d.%d-x86_32 ", major, minor); + p += i; + i = snprintf(p, sizeof(xen_capabilities_info_t) - i, + "hvm-%d.%d-x86_32p ", major, minor); + p += i; } #elif defined(CONFIG_X86_64) - p += sprintf(p, "xen-%d.%d-x86_64 ", major, minor); + i = snprintf(p, sizeof(xen_capabilities_info_t), + "xen-%d.%d-x86_64 ", major, minor); + p += i; #ifdef CONFIG_COMPAT - p += sprintf(p, "xen-%d.%d-x86_32p ", major, minor); + i = snprintf(p, sizeof(xen_capabilities_info_t) - i, + "xen-%d.%d-x86_32p ", major, minor); + p += i; #endif if ( hvm_enabled ) { - p += sprintf(p, "hvm-%d.%d-x86_32 ", major, minor); - p += sprintf(p, "hvm-%d.%d-x86_32p ", major, minor); - p += sprintf(p, "hvm-%d.%d-x86_64 ", major, minor); + i = snprintf(p, sizeof(xen_capabilities_info_t) - i, + "hvm-%d.%d-x86_32 ", major, minor); + p += i; + i = snprintf(p, sizeof(xen_capabilities_info_t) - i, + "hvm-%d.%d-x86_32p ", major, minor); + p += i; + i = snprintf(p, sizeof(xen_capabilities_info_t) - i, + "hvm-%d.%d-x86_64 ", major, minor); + p += i; } #else diff --git a/xen/arch/x86/time.c b/xen/arch/x86/time.c index 387b18c20a..b57f34f4f0 100644 --- a/xen/arch/x86/time.c +++ b/xen/arch/x86/time.c @@ -274,7 +274,7 @@ static char *freq_string(u64 freq) unsigned int x, y; y = (unsigned int)do_div(freq, 1000000) / 1000; x = (unsigned int)freq; - sprintf(s, "%u.%03uMHz", x, y); + snprintf(s, sizeof(s), "%u.%03uMHz", x, y); return s; } diff --git a/xen/common/gdbstub.c b/xen/common/gdbstub.c index 8c863080e0..b9dfd08bbc 100644 --- a/xen/common/gdbstub.c +++ b/xen/common/gdbstub.c @@ -268,7 +268,7 @@ gdb_send_packet(struct gdb_context *ctx) char buf[3]; int count; - sprintf(buf, "%.02x\n", ctx->out_csum); + snprintf(buf, sizeof(buf), "%.02x\n", ctx->out_csum); gdb_write_to_packet_char('#', ctx); gdb_write_to_packet(buf, 2, ctx); diff --git a/xen/common/kernel.c b/xen/common/kernel.c index 016270b2c5..4c22cf8803 100644 --- a/xen/common/kernel.c +++ b/xen/common/kernel.c @@ -72,8 +72,7 @@ void cmdline_parse(char *cmdline) switch ( param->type ) { case OPT_STR: - strncpy(param->var, optval, param->len); - ((char *)param->var)[param->len-1] = '\0'; + strlcpy(param->var, optval, param->len); break; case OPT_UINT: *(unsigned int *)param->var = diff --git a/xen/common/keyhandler.c b/xen/common/keyhandler.c index 244273addf..5ff45d0d54 100644 --- a/xen/common/keyhandler.c +++ b/xen/common/keyhandler.c @@ -67,7 +67,7 @@ void register_keyhandler( ASSERT(key_table[key].u.handler == NULL); key_table[key].u.handler = handler; key_table[key].flags = 0; - strncpy(key_table[key].desc, desc, STR_MAX); + strlcpy(key_table[key].desc, desc, STR_MAX); key_table[key].desc[STR_MAX-1] = '\0'; } @@ -77,8 +77,7 @@ void register_irq_keyhandler( ASSERT(key_table[key].u.irq_handler == NULL); key_table[key].u.irq_handler = handler; key_table[key].flags = KEYHANDLER_IRQ_CALLBACK; - strncpy(key_table[key].desc, desc, STR_MAX); - key_table[key].desc[STR_MAX-1] = '\0'; + strlcpy(key_table[key].desc, desc, STR_MAX); } static void show_handlers(unsigned char key) diff --git a/xen/common/libelf/libelf-dominfo.c b/xen/common/libelf/libelf-dominfo.c index c19339ab01..d3ddcdb6fe 100644 --- a/xen/common/libelf/libelf-dominfo.c +++ b/xen/common/libelf/libelf-dominfo.c @@ -128,16 +128,16 @@ int elf_xen_parse_note(struct elf_binary *elf, switch (type) { case XEN_ELFNOTE_LOADER: - strncpy(parms->loader, str, sizeof(parms->loader)); + strlcpy(parms->loader, str, sizeof(parms->loader)); break; case XEN_ELFNOTE_GUEST_OS: - strncpy(parms->guest_os, str, sizeof(parms->guest_os)); + strlcpy(parms->guest_os, str, sizeof(parms->guest_os)); break; case XEN_ELFNOTE_GUEST_VERSION: - strncpy(parms->guest_ver, str, sizeof(parms->guest_ver)); + strlcpy(parms->guest_ver, str, sizeof(parms->guest_ver)); break; case XEN_ELFNOTE_XEN_VERSION: - strncpy(parms->xen_ver, str, sizeof(parms->xen_ver)); + strlcpy(parms->xen_ver, str, sizeof(parms->xen_ver)); break; case XEN_ELFNOTE_PAE_MODE: if (0 == strcmp(str, "yes")) @@ -224,13 +224,13 @@ int elf_xen_parse_guest_info(struct elf_binary *elf, /* strings */ if (0 == strcmp(name, "LOADER")) - strncpy(parms->loader, value, sizeof(parms->loader)); + strlcpy(parms->loader, value, sizeof(parms->loader)); if (0 == strcmp(name, "GUEST_OS")) - strncpy(parms->guest_os, value, sizeof(parms->guest_os)); + strlcpy(parms->guest_os, value, sizeof(parms->guest_os)); if (0 == strcmp(name, "GUEST_VER")) - strncpy(parms->guest_ver, value, sizeof(parms->guest_ver)); + strlcpy(parms->guest_ver, value, sizeof(parms->guest_ver)); if (0 == strcmp(name, "XEN_VER")) - strncpy(parms->xen_ver, value, sizeof(parms->xen_ver)); + strlcpy(parms->xen_ver, value, sizeof(parms->xen_ver)); if (0 == strcmp(name, "PAE")) { if (0 == strcmp(value, "yes[extended-cr3]")) diff --git a/xen/common/perfc.c b/xen/common/perfc.c index 5ab8c6e9f3..86fda48e07 100644 --- a/xen/common/perfc.c +++ b/xen/common/perfc.c @@ -148,9 +148,8 @@ static int perfc_copy_info(XEN_GUEST_HANDLE_64(xen_sysctl_perfc_desc_t) desc, { for ( i = 0; i < NR_PERFCTRS; i++ ) { - strncpy(perfc_d[i].name, perfc_info[i].name, + strlcpy(perfc_d[i].name, perfc_info[i].name, sizeof(perfc_d[i].name)); - perfc_d[i].name[sizeof(perfc_d[i].name)-1] = '\0'; switch ( perfc_info[i].type ) { diff --git a/xen/common/rangeset.c b/xen/common/rangeset.c index d90a39b67b..3594db41e9 100644 --- a/xen/common/rangeset.c +++ b/xen/common/rangeset.c @@ -283,12 +283,11 @@ struct rangeset *rangeset_new( if ( name != NULL ) { - strncpy(r->name, name, sizeof(r->name)); - r->name[sizeof(r->name)-1] = '\0'; + strlcpy(r->name, name, sizeof(r->name)); } else { - sprintf(r->name, "(no name)"); + snprintf(r->name, sizeof(r->name), "(no name)"); } if ( (r->domain = d) != NULL ) diff --git a/xen/common/symbols.c b/xen/common/symbols.c index fba6cf0867..f4134b7ed5 100644 --- a/xen/common/symbols.c +++ b/xen/common/symbols.c @@ -142,15 +142,17 @@ void __print_symbol(const char *fmt, unsigned long address) const char *name; unsigned long offset, size; char namebuf[KSYM_NAME_LEN+1]; - char buffer[sizeof("%s+%#lx/%#lx [%s]") + KSYM_NAME_LEN + - 2*(BITS_PER_LONG*3/10) + 1]; + +#define BUFFER_SIZE sizeof("%s+%#lx/%#lx [%s]") + KSYM_NAME_LEN + \ + 2*(BITS_PER_LONG*3/10) + 1 + char buffer[BUFFER_SIZE]; name = symbols_lookup(address, &size, &offset, namebuf); if (!name) - sprintf(buffer, "???"); + snprintf(buffer, BUFFER_SIZE, "???"); else - sprintf(buffer, "%s+%#lx/%#lx", name, offset, size); + snprintf(buffer, BUFFER_SIZE, "%s+%#lx/%#lx", name, offset, size); printk(fmt, buffer); } -- 2.30.2